Make explicit guarantees about Vec’s allocator
#145260
Conversation
This commit amends the documentation of `Vec::as_mut_ptr` and `Vec::into_raw_parts` to make it explicit that such calls may be paired with calls to `dealloc` with a suitable layout. This guarantee was effectively already provided by the docs of `Vec::from_raw_parts` mentioning `alloc`. Additionally, we copy-paste and adjust the “Memory layout” section from the documentation of `std::boxed` to `std::vec`. This explains the allocator guarantees in more detail.
|
r? @ibraheemdev rustbot has assigned @ibraheemdev. Use |
rustbot
added
S-waiting-on-review
|
r? libs-api |
rustbot
added
the
T-libs-api
There was a problem hiding this comment.
This guarantee was effectively already provided by the docs of
Vec::from_raw_partsmentioningalloc.
This is also my understanding. If you can feed an arbitrary heap allocation into Vec::from_raw_parts and get it back from into_raw_parts / as_mut_ptr / capacity, it's effectively a guarantee that these things can be deallocated in the obvious way.
There is one other piece required to complete the argument. The above by itself does not guarantee that a ZST vector ptr is fine to "leak" as written in this PR: "and if T is zero-sized nothing needs to be done". This scenario falls outside any consequences drawn from from_raw_parts because from_raw_parts is documented as follows: "ptr must have been allocated using the global allocator, such as via the alloc::alloc function", and allocating a zero-sized layout with a global allocator is documented as being UB. If Vec somehow allocated nonzero size internally for ZST vectors, this PR would be a new guarantee. But there is plenty of existing documentation that Vec does not allocate for ZST.
Hypothetically, dropping a vector of ZST could require some other non-allocator thing, such as std::internal::decrement_outstanding_zst_vectors(). But this is pretty outlandish and I do not believe we require a team consensus to rule this out.
Thank you for the improvements!
Separately, it would be good to update the documentation of from_raw_parts to speak about the ZST case. As currently written, it seems to imply round-tripping into_raw_parts -> from_raw_parts can be UB, which is not the intention.
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
…r=dtolnay Make explicit guarantees about `Vec`’s allocator This commit amends the documentation of `Vec::as_mut_ptr` and `Vec::into_raw_parts` to make it explicit that such calls may be paired with calls to `dealloc` with a suitable layout. This guarantee was effectively already provided by the docs of `Vec::from_raw_parts` mentioning `alloc`. Additionally, we copy-paste and adjust the “Memory layout” section from the documentation of `std::boxed` to `std::vec`. This explains the allocator guarantees in more detail.
…r=dtolnay Make explicit guarantees about `Vec`’s allocator This commit amends the documentation of `Vec::as_mut_ptr` and `Vec::into_raw_parts` to make it explicit that such calls may be paired with calls to `dealloc` with a suitable layout. This guarantee was effectively already provided by the docs of `Vec::from_raw_parts` mentioning `alloc`. Additionally, we copy-paste and adjust the “Memory layout” section from the documentation of `std::boxed` to `std::vec`. This explains the allocator guarantees in more detail.
…r=dtolnay Make explicit guarantees about `Vec`’s allocator This commit amends the documentation of `Vec::as_mut_ptr` and `Vec::into_raw_parts` to make it explicit that such calls may be paired with calls to `dealloc` with a suitable layout. This guarantee was effectively already provided by the docs of `Vec::from_raw_parts` mentioning `alloc`. Additionally, we copy-paste and adjust the “Memory layout” section from the documentation of `std::boxed` to `std::vec`. This explains the allocator guarantees in more detail.
…r=dtolnay Make explicit guarantees about `Vec`’s allocator This commit amends the documentation of `Vec::as_mut_ptr` and `Vec::into_raw_parts` to make it explicit that such calls may be paired with calls to `dealloc` with a suitable layout. This guarantee was effectively already provided by the docs of `Vec::from_raw_parts` mentioning `alloc`. Additionally, we copy-paste and adjust the “Memory layout” section from the documentation of `std::boxed` to `std::vec`. This explains the allocator guarantees in more detail.
|
This PR is incorrect. It needs to mention the case where the Vec is zero-length in addition to the case where T is zero-sized. |
Rollup of 15 pull requests Successful merges: - #131477 (Apple: Always pass SDK root when linking with `cc`, and pass it via `SDKROOT` env var) - #139806 (std: sys: pal: uefi: Overhaul Time) - #144386 (Extract TraitImplHeader in AST/HIR) - #144542 (Stabilize `sse4a` and `tbm` target features) - #144921 (Don't emit `rustdoc::broken_intra_doc_links` for GitHub-flavored Markdown admonitions like `[!NOTE]`) - #145155 (Port `#[allow_internal_unsafe]` to the new attribute system (attempt 2)) - #145214 (fix: re-enable self-assignment) - #145216 (rustdoc: correct negative-to-implicit discriminant display) - #145238 (Tweak invalid builtin attribute output) - #145249 (Rename entered trace span variables from `_span` to `_trace`) - #145251 (Support using #[unstable_feature_bound] on trait) - #145253 (Document compiler and stdlib in stage1 in `pr-check-2` CI job) - #145260 (Make explicit guarantees about `Vec`’s allocator) - #145263 (Update books) - #145273 (Account for new `assert!` desugaring in `!condition` suggestion) r? `@ghost` `@rustbot` modify labels: rollup
|
The overarching rollup is 20mins in (out if ~3h15 total). If theemathas's concern is valid, either r- now-ish or wait for the rollup to fail or get merged (in the latter case: plz open a follow-up PR instead). Either way, don't push before an r-. |
|
Ok, rollup failed. I'll tangentially r- this due to the concern raised in #145260 (comment) (of which I don't know if it is valid). |
bors
added
S-waiting-on-author
|
The comment is correct – that’s an oversight on my part. I also updated the docs of |
|
@bors r+ |
bors
added
S-waiting-on-bors
…r=dtolnay Make explicit guarantees about `Vec`’s allocator This commit amends the documentation of `Vec::as_mut_ptr` and `Vec::into_raw_parts` to make it explicit that such calls may be paired with calls to `dealloc` with a suitable layout. This guarantee was effectively already provided by the docs of `Vec::from_raw_parts` mentioning `alloc`. Additionally, we copy-paste and adjust the “Memory layout” section from the documentation of `std::boxed` to `std::vec`. This explains the allocator guarantees in more detail.
Rollup of 17 pull requests Successful merges: - #131477 (Apple: Always pass SDK root when linking with `cc`, and pass it via `SDKROOT` env var) - #139806 (std: sys: pal: uefi: Overhaul Time) - #144210 (std: thread: Return error if setting thread stack size fails) - #144386 (Extract TraitImplHeader in AST/HIR) - #144921 (Don't emit `rustdoc::broken_intra_doc_links` for GitHub-flavored Markdown admonitions like `[!NOTE]`) - #145155 (Port `#[allow_internal_unsafe]` to the new attribute system (attempt 2)) - #145214 (fix: re-enable self-assignment) - #145216 (rustdoc: correct negative-to-implicit discriminant display) - #145238 (Tweak invalid builtin attribute output) - #145249 (Rename entered trace span variables from `_span` to `_trace`) - #145251 (Support using #[unstable_feature_bound] on trait) - #145253 (Document compiler and stdlib in stage1 in `pr-check-2` CI job) - #145260 (Make explicit guarantees about `Vec`’s allocator) - #145263 (Update books) - #145273 (Account for new `assert!` desugaring in `!condition` suggestion) - #145283 (Make I-miscompile imply I-prioritize) - #145291 (bootstrap: Only warn about `rust.debug-assertions` if downloading rustc) r? `@ghost` `@rustbot` modify labels: rollup
This commit amends the documentation of
Vec::as_mut_ptrandVec::into_raw_partsto make it explicit that such calls may be paired with calls todeallocwith a suitable layout. This guarantee was effectively already provided by the docs ofVec::from_raw_partsmentioningalloc.Additionally, we copy-paste and adjust the “Memory layout” section from the documentation of
std::boxedtostd::vec. This explains the allocator guarantees in more detail.